feat(capture): add AMD Display Capture for AFMF #3171

radugrecu97 · 2024-09-11T19:19:42Z

Description

This PR adds a new capture option under advanced settings - AMD Display capture.
The main purpose of this new capture method would be to capture AFMF frames.

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Dependency update (updates to dependencies)
Documentation update (changes to documentation)
Repository update (changes to repository files, e.g. .github/...)

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Known bugs

Mouse isn't captured unless it's of a very large size
When using AFMF, frames might be captured out of order during and could result in temporary "ghosted" image. Some testing showed that it's caused by display changes and moonlight pause and resume. This is fixed after disabling and re-enabling AFMF or sometimes waiting. Different capturing mode parameters might fix this issue and a proper testing methodology should prove the current best approach.

Known limitations

AMD card is required to use this capturing mode, regardless of use of AFMF
AFMF can't be used with VDD because it Adrenaline doesn't recognize the virtual display and therefore no special features can be used. So a monitor or edid emulator has to be used as the capturing display.

To do:

Fix mouse cursor streaming
Add screenshot showing new UI option
Fix quality check
Maybe add a new field for changing Display Capture, capturing mode parameter.

ReenigneArcher

Thank you for the PR. This will be a nice addition!

src/platform/windows/display.h

ReenigneArcher · 2024-09-30T00:00:24Z

src/platform/windows/display_amd.cpp

+   /**
+   * @brief Get the next frame from the producer thread.
+   * If not available, the capture thread blocks until one is, or the wait times out.
+   * @param timeout how long to wait for the next frame
+   * @param out a texture containing the frame just captured
+   * @param out_time the timestamp of the frame just captured
+   */


Suggested change

/**

* @brief Get the next frame from the producer thread.

* If not available, the capture thread blocks until one is, or the wait times out.

* @param timeout how long to wait for the next frame

* @param out a texture containing the frame just captured

* @param out_time the timestamp of the frame just captured

*/

/**

* @brief Get the next frame from the producer thread.

* If not available, the capture thread blocks until one is, or the wait times out.

* @param timeout how long to wait for the next frame

* @param out a texture containing the frame just captured

* @param out_time the timestamp of the frame just captured

*/

This should also probably be moved to the header (doxygen can complain if it exists in the header and implementation if there is any mismatch).

Not sure what you mean here. I fixed the misaligned comment.

I mean to move the documentation block to the .h file instead of the .cpp file.

ReenigneArcher · 2024-09-30T00:04:04Z

src/platform/windows/display_base.cpp

@@ -1051,6 +1053,16 @@ namespace platf {
      }
    }

+    if (config::video.capture == "amd") {


Suggested change

if (config::video.capture == "amd") {

if (config::video.capture == "amd" || config::video.capture.empty()) {

This should allot it to attempt amd capture if the user hasn't explicitly set anything.

What should be our automatic order? The way it is now will try in this order:

ddx

amd

wgc

I moved amd first as AMD users would prefer it over ddx because it supports AFMF.

Only downside right now is the mouse cursor not being captured.

I think it will be fine to be first once the issues like this are ironed out. No mouse capture is a pretty big downside at the moment.

src/platform/windows/display_vram.cpp

src/stream.cpp

src/video.cpp

src_assets/common/assets/web/configs/tabs/Advanced.vue

* Drain queued frames before termination

* Remove AMD Trace and debug statements * Add code comments

sonarqubecloud · 2024-10-09T19:21:05Z

Quality Gate failed

Failed conditions
10 New issues
9.8% Duplication on New Code (required ≤ 2%)
2 Duplicated Blocks on New Code (required ≤ 0)
10 New Code Smells (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

radugrecu97 · 2024-10-10T17:05:26Z

So the latest AMD driver seems to have had a quality regression for AFMF 2 compared to the preview.

I'll test again later but AFMF 2 looked so blurry and jittery on the latest drivers.

ReenigneArcher · 2024-10-10T17:49:39Z

So the latest AMD driver seems to have had a quality regression for AFMF 2 compared to the preview.

I'll test again later but AFMF 2 looked so blurry and jittery when streamed compared to the preview.

Possibly another regression exposed with the ffmpeg 7.1 update?

codecov · 2024-10-10T18:05:16Z

Codecov Report

Attention: Patch coverage is 0% with 130 lines in your changes missing coverage. Please review.

Project coverage is 7.56%. Comparing base (d378c18) to head (38763bb).
Report is 15 commits behind head on master.

Files with missing lines	Patch %	Lines
src/platform/windows/display_amd.cpp	0.00%	82 Missing ⚠️
src/platform/windows/display_vram.cpp	0.00%	38 Missing ⚠️
src/platform/windows/display_base.cpp	0.00%	7 Missing ⚠️
src/platform/windows/display.h	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           master   #3171      +/-   ##
=========================================
- Coverage    9.74%   7.56%   -2.18%     
=========================================
  Files         101     102       +1     
  Lines       17975   18038      +63     
  Branches     8420    8477      +57     
=========================================
- Hits         1751    1364     -387     
- Misses      13330   13858     +528     
+ Partials     2894    2816      -78

Flag	Coverage Δ
Linux	`7.27% <ø> (ø)`
Windows	`0.00% <0.00%> (-5.11%)`	⬇️
macOS-13	`10.54% <ø> (-0.04%)`	⬇️
macOS-14	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/video.cpp	`26.15% <ø> (-1.03%)`	⬇️
src/platform/windows/display.h	`0.00% <0.00%> (-7.25%)`	⬇️
src/platform/windows/display_base.cpp	`0.00% <0.00%> (-13.60%)`	⬇️
src/platform/windows/display_vram.cpp	`0.00% <0.00%> (-1.85%)`	⬇️
src/platform/windows/display_amd.cpp	`0.00% <0.00%> (ø)`

... and 39 files with indirect coverage changes

radugrecu97 · 2024-10-11T07:52:38Z

So the latest AMD driver seems to have had a quality regression for AFMF 2 compared to the preview.
I'll test again later but AFMF 2 looked so blurry and jittery when streamed compared to the preview.

Possibly another regression exposed with the ffmpeg 7.1 update?

FFmpeg 7.1 changelog doesn't tell me much, BUT I was surprised that a different Display Capture mode is not displaying frames out of order any more compared to previous experience.

The AMF sdk release does mention that they updated ffmpeg to 7.0, so maybe they didn't test with 7.1.

This comment was marked as off-topic.

Sign in to view

This comment was marked as resolved.

Sign in to view

ReenigneArcher requested changes Sep 30, 2024

View reviewed changes

This comment was marked as off-topic.

Sign in to view

radugrecu97 requested a review from ReenigneArcher October 9, 2024 19:04

radugrecu97 force-pushed the feat/amd_dispay_capture2 branch from cbe742c to 708db84 Compare October 9, 2024 19:07

radugrecu97 added 3 commits October 9, 2024 21:12

AMD Display Capture

e0adedb

AMD Display Capture reinit fix

6f154df

* Drain queued frames before termination

AMD Display Capture documentation and fixes

38763bb

* Remove AMD Trace and debug statements * Add code comments

radugrecu97 force-pushed the feat/amd_dispay_capture2 branch from 708db84 to 38763bb Compare October 9, 2024 19:12

ReenigneArcher changed the title ~~AMD Display Capture for AFMF~~ feat(capture): add AMD Display Capture for AFMF Oct 10, 2024

ReenigneArcher marked this pull request as draft December 16, 2024 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(capture): add AMD Display Capture for AFMF #3171

feat(capture): add AMD Display Capture for AFMF #3171

radugrecu97 commented Sep 11, 2024 •

edited

Loading

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as resolved.

ReenigneArcher left a comment

ReenigneArcher Sep 30, 2024 •

edited

Loading

radugrecu97 Oct 9, 2024

ReenigneArcher Oct 10, 2024

ReenigneArcher Sep 30, 2024

radugrecu97 Oct 9, 2024

ReenigneArcher Oct 10, 2024

This comment was marked as off-topic.

sonarqubecloud bot commented Oct 9, 2024

radugrecu97 commented Oct 10, 2024 •

edited

Loading

ReenigneArcher commented Oct 10, 2024

codecov bot commented Oct 10, 2024 •

edited

Loading

radugrecu97 commented Oct 11, 2024

	if (config::video.capture == "amd") {
	if (config::video.capture == "amd" \|\| config::video.capture.empty()) {

feat(capture): add AMD Display Capture for AFMF #3171

Are you sure you want to change the base?

feat(capture): add AMD Display Capture for AFMF #3171

Conversation

radugrecu97 commented Sep 11, 2024 • edited Loading

Description

Type of Change

Checklist

Known bugs

Known limitations

To do:

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as resolved.

ReenigneArcher left a comment

Choose a reason for hiding this comment

ReenigneArcher Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

radugrecu97 Oct 9, 2024

Choose a reason for hiding this comment

ReenigneArcher Oct 10, 2024

Choose a reason for hiding this comment

ReenigneArcher Sep 30, 2024

Choose a reason for hiding this comment

radugrecu97 Oct 9, 2024

Choose a reason for hiding this comment

ReenigneArcher Oct 10, 2024

Choose a reason for hiding this comment

This comment was marked as off-topic.

sonarqubecloud bot commented Oct 9, 2024

Quality Gate failed

radugrecu97 commented Oct 10, 2024 • edited Loading

ReenigneArcher commented Oct 10, 2024

codecov bot commented Oct 10, 2024 • edited Loading

Codecov Report

radugrecu97 commented Oct 11, 2024

radugrecu97 commented Sep 11, 2024 •

edited

Loading

ReenigneArcher Sep 30, 2024 •

edited

Loading

radugrecu97 commented Oct 10, 2024 •

edited

Loading

codecov bot commented Oct 10, 2024 •

edited

Loading